refactor: use sourcegraph/run for command execution#127
Merged
Conversation
Replace custom os/exec-based command execution with github.com/sourcegraph/run. The new implementation delegates process lifecycle management to the library while preserving the existing public API and all behavioral contracts (timeout, cancellation, stderr handling, debug logging). Key changes in command.go: - Replace exec.CommandContext with run.Cmd, using run.Arg to quote arguments for the shell-based parser - Add newRunCmd helper to centralize run.Command construction - Simplify RunInDirWithOptions by streaming stdout via Output.Stream and extracting stderr from the error - Optimize RunInDir to stream directly to a bytes.Buffer, preserving raw bytes (important for binary-ish output like ls-tree -z)
Convert repo_tree.go LsTree to use gitRun with local args, removing the last NewCommand call site. Rewrite command_test.go to test the new gitRun/gitPipeline helpers directly, removing tests for the deleted Command struct.
Use opt.Args/opt.Envs instead of opt.CommandOptions.Args/opt.CommandOptions.Envs since CommandOptions is an embedded field.
Fix timeout context cancel leakage, tighten exit-status matching, remove fragile stderr re-concatenation paths, and harden author/diff argument handling.
This comment was marked as resolved.
This comment was marked as resolved.
- Fix exit status matching: replace loose HasPrefix with isExitStatus helper that won't match "exit status 1" against "exit status 128" - Fix opt.Args ordering in ShowRefVerify and DeleteTag: move before --end-of-options so flag args work correctly - Fix gitRun log capture: capture partial stdout on error instead of logging empty output for failed commands - Quote args with special characters in log output for readability - Fix repo_pull_test: "bad_revision" causes exit 128, not exit 1 Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
unknwon
commented
Feb 15, 2026
- Remove nil check on ctx in gitCmd (callers always provide non-nil ctx) - Remove stdin parameter from gitPipeline (no production caller uses it) - Rewrite isExitStatus to use run.ExitCoder interface instead of string parsing, as suggested by reviewer - Remove unused errors and os/exec imports - Update tests to use gitCmd directly for stdin-based scenarios Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
unknwon
commented
Feb 15, 2026
- Rename helpers: gitCmd -> cmd, gitRun -> exec, gitPipeline -> pipe - Remove stderr parameter from pipe (stderr is already in the error) - Remove extractStderr (no longer used after stderr removal) - Remove Blob.Pipeline stderr parameter and update all callers - Rename CreateArchive -> Archive and add --end-of-options Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
unknwon
commented
Feb 15, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
os/exec-based command execution incommand.gowithgithub.com/sourcegraph/run, delegating process lifecycle management to the libraryRunInDir,RunInDirPipeline,RunInDirWithOptions,Run) and all behavioral contracts (default timeout, cancellation →context.Canceled, deadline →ErrExecTimeout, stderr in errors, debug logging)run.Arg()to properly quote arguments for the library's shell-based parser, andOutput.Stream()for raw byte-preserving output (important for binary-ish output likels-tree -z)All existing tests pass without modification.